Skip to content

codemod iterations#2386

Merged
KKonstantinov merged 7 commits into
mainfrom
feature/codemod-iterations-4
Jun 29, 2026
Merged

codemod iterations#2386
KKonstantinov merged 7 commits into
mainfrom
feature/codemod-iterations-4

Conversation

@KKonstantinov

@KKonstantinov KKonstantinov commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

feat(codemod): test the batch harness against the published codemod + SDK, and preserve shebangs

Extends the codemod batch-test harness so it can validate the published @modelcontextprotocol/codemod and the published v2 SDK packages from npm — not just the local working copy — and fixes a codemod bug that dropped a file's leading #! shebang during migration.

Motivation and Context

The batch-test harness runs the v1→v2 codemod against real-world repos to catch regressions, missing transforms, and gaps. Until now it could only exercise the local working-copy codemod against locally-packed SDK tarballs. That validates the branch under development, but never the artifacts users actually npm install — so a bug present only in a published build, or an incompatibility with a published SDK alpha, would slip through.

This PR adds independent source selection for the codemod and the SDK, each local or published, plus version pinning:

Flag Values Default Effect
--codemod local | published local local: run the working-copy codemod in-process. published: npx the published CLI.
--sdk local | published local local: pack local packages, rewrite deps to file: tarballs. published: install v2 deps from npm.
--codemod-version <spec> npm version/tag/range latest Only with --codemod=published.
--sdk-version <spec> npm version/tag/range (unset) Only with --sdk=published. Pins each v2 dep to its own resolved version; unset = whatever the codemod writes.

So the same harness can now answer "does the published codemod + published SDK still migrate these repos cleanly?", with results written to per-run directories keyed on the resolved versions.

Separately, several transforms consumed a file's leading #! shebang (it is leading trivia of the first import they rewrite), silently breaking CLI packages whose bin points at the migrated entry. The runner now captures the shebang before transforms and restores it before saving.

How Has This Been Tested?

  • New unit suite test/batchTest.test.ts — arg parsing (--flag value and --flag=value, -- separator, invalid/unknown flags, ignored version overrides), parseNpmViewVersion, parseCodemodCliOutput, computeResultsDirName across all modes, rewriteToPublishedVersion (per-package independent versions, formatting/trailing-newline preserved), cleanSubprocessEnv, and local runCodemod.
  • Shebang regression teststest/integration.test.ts verifies a migrated CLI file keeps #!/usr/bin/env node (plus its following blank line); test/commentInsertion.test.ts verifies a comment-bearing diagnostic's reported file:line points at the actual line in the saved (shebang-restored) file.
  • End-to-end — ran the harness against the repos in repos.json (punkpeye/fastmcp, upstash/context7, firebase/firebase-tools) in both local and published modes.
  • pnpm check:all is green (typecheck + lint + docs:check); the full codemod suite passes (411 tests).

Breaking Changes

None. The batch-test harness is dev-only tooling. The shebang fix is a user-facing codemod bugfix (output is now more correct) shipped as a patch changeset for @modelcontextprotocol/codemod; no consumer action required.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Results layout. Each run writes to batch-test/results/codemod-…__sdk-…/, keyed on the resolved versions, with a top-level summary.json (per-repo pass/fail, error counts, run config, and the resolved per-package sdkVersions) and a report.json per repo (baseline vs post-codemod check results, diagnostics, change counts). In published-codemod mode the CLI emits text rather than structured diagnostics, so codemod.diagnostics is empty and the raw CLI output is captured under codemod.cli.

Subprocess env hygiene. The harness is invoked via pnpm, which exports its own config as npm_config_* / PNPM_* vars; those leak into every subprocess and break things (minimum-release-age blocks installs, workspace-cwd npx mis-resolves). cleanSubprocessEnv strips them so installs, npx, and npm view see a clean package-manager env (npmrc files still honored).

Security. The npx / npm view / git shell-outs interpolate only operator-controlled inputs — CLI flags, the committed repos.json, and Anthropic-published npm versions. JSON.stringify quoting does not neutralize $(…)/backticks under sh -c, so the harness must never be pointed at an untrusted manifest. This is called out at each call site.

Review fixes folded in (latest commit):

  • typedoc.json excludes the dev-only src/bin/** so the harness's exported types stop tripping typedoc's treatWarningsAsErrors (this was failing docs:check, hence check:all).
  • Post-install "actually-installed" versions are recorded into a separate map from the startup-resolved pins, so a package left unpinned at startup isn't retroactively pinned for later repos from an earlier repo's install.
  • Corrected the npm view "aborts the run" wording (a per-package --sdk-version miss only warns) and the parseNpmViewVersion comment (npm lists matches in publish order, not semver order).

@KKonstantinov KKonstantinov requested a review from a team as a code owner June 29, 2026 14:47
@changeset-bot

changeset-bot Bot commented Jun 29, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 85879d7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@modelcontextprotocol/codemod Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Jun 29, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2386

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2386

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2386

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2386

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2386

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2386

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2386

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2386

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2386

commit: 85879d7

@felixweinberger felixweinberger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a CI check - seems to fail a build

Comment thread packages/codemod/batch-test/README.md Outdated
Comment thread packages/codemod/src/runner.ts
Comment thread packages/codemod/src/bin/batchTest.ts
Comment thread packages/codemod/src/runner.ts
Comment thread packages/codemod/src/bin/batchTest.ts
Comment thread packages/codemod/batch-test/README.md Outdated
  - typedoc: exclude src/bin from codemod docs so the batch-test harness's
    exported types no longer trip treatWarningsAsErrors (fixes check:all)
  - runner: shift diagnostic lines by the restored shebang's line count so
    reported file:line matches the saved file
  - batchTest: record post-install versions in a separate map so a
    startup-unresolved package isn't retroactively pinned for later repos
  - batchTest/README: correct npm-view abort wording, the parseNpmViewVersion
    'highest match' comment, and 'How it works' step 4
  - changeset: patch @modelcontextprotocol/codemod for the shebang fix
…tprotocol/typescript-sdk into feature/codemod-iterations-4
Comment thread packages/codemod/src/runner.ts
@KKonstantinov

Copy link
Copy Markdown
Contributor Author

@claude review

Comment thread packages/codemod/batch-test/repos.json Outdated
@KKonstantinov KKonstantinov merged commit 36055d5 into main Jun 29, 2026
18 checks passed
@KKonstantinov KKonstantinov deleted the feature/codemod-iterations-4 branch June 29, 2026 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants